Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ConvTranspose symmetric non-constant padding #2463

Merged
merged 1 commit into from
Aug 1, 2024

Conversation

paulnovo
Copy link
Contributor

The ConvTranspose was not able to handle symmetric non-constant padding (ie, pad=(1, 0) for 2D ConvTranspose). Constant padding (ie pad=1 for 2D ConvTranspose) and assymetric non-constant padding (ie, pad=(1, 0, 2, 3)) worked correctly. This commit fixes symmetric non-constant padding and adds unit tests to ensure it produces the same output size as an equivalent fully expanded padding.

Fixes #2424

PR Checklist

  • Tests are added
  • Entry in NEWS.md
  • Documentation, if applicable

Copy link

codecov bot commented Jun 22, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes missing coverage. Please review.

Project coverage is 73.91%. Comparing base (36abc73) to head (2af2862).

Current head 2af2862 differs from pull request most recent head 1dbbf52

Please upload reports for the commit 1dbbf52 to get more accurate results.

Files Patch % Lines
ext/FluxAMDGPUExt/conv.jl 0.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #2463       +/-   ##
===========================================
+ Coverage   40.98%   73.91%   +32.92%     
===========================================
  Files          32       32               
  Lines        1886     1928       +42     
===========================================
+ Hits          773     1425      +652     
+ Misses       1113      503      -610     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@CarloLucibello
Copy link
Member

looks good. Could you rebase?

The ConvTranspose was not able to handle symmetric non-constant
padding (ie, `pad=(1, 0)` for 2D ConvTranspose). Constant padding (ie
`pad=1` for 2D ConvTranspose) and assymetric non-constant padding (ie,
`pad=(1, 0, 2, 3)`) worked correctly. This commit fixes symmetric
non-constant padding and adds unit tests to ensure it produces the same
output size as an equivalent fully expanded padding.

Fixes FluxML#2424
@paulnovo paulnovo force-pushed the convtranspose-symmetric-pad branch from 1dbbf52 to 597ec6f Compare July 28, 2024 13:15
@paulnovo
Copy link
Contributor Author

@CarloLucibello I believe the AMD test failures on buildkite are also failing on master. I was able to get all AMD tests passing on a branch without the recent removal of enzyme. Can this merge, or should we wait for everything to pass.

@CarloLucibello CarloLucibello merged commit 7c1ef13 into FluxML:master Aug 1, 2024
5 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ConvTranspose errors with symmetric non-constant pad
2 participants